Fix Multitenancy Support and Normalize Tenant ID Handling#7217
Fix Multitenancy Support and Normalize Tenant ID Handling#7217sfmskywalker merged 27 commits intorelease/3.6.0from
Conversation
- Activate multitenancy in `Program.cs`. - Introduce `NormalizeTenantId` method for consistent tenant ID usage. - Update tenant-related classes and features to support normalization logic.
- Standardized the tenant ID for the default tenant to use an empty string (`""`) instead of `null`. - Documented the rationale and migration considerations in ADR 0007. - Updated ADR table of contents and graph for new entry.
There was a problem hiding this comment.
Pull request overview
This PR standardizes the multitenancy system on using an empty string ("") as the default tenant ID instead of null, and documents the decision as an ADR. It introduces a normalization helper to keep behavior backward compatible while fixing dictionary usage issues and wiring multitenancy into the sample server.
Changes:
- Define
Tenant.DefaultTenantId = ""and update the default tenant to use this constant. - Add a
NormalizeTenantId()extension and use it in tenant resolution code to safely handlenullIDs in dictionaries and lookups. - Enable multitenancy in
Elsa.Server.Web, and add ADR 0007 plus updates to the ADR TOC and graph to record the architectural decision.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs |
Normalizes tenant IDs when building the tenant dictionary and when resolving tenant IDs, preventing null keys and aligning with the new default ID convention. |
src/modules/Elsa.Tenants/Extensions/ModuleExtensions.cs |
Adds [UsedImplicitly] attributes and updates the tenants module extension, but currently introduces invalid syntax around tenant management extension methods that will prevent compilation. |
src/modules/Elsa.Common/Multitenancy/Extensions/TenantsProviderExtensions.cs |
Introduces NormalizeTenantId(this string? tenantId) to convert null to Tenant.DefaultTenantId, centralizing the default tenant ID normalization logic. |
src/modules/Elsa.Common/Multitenancy/Entities/Tenant.cs |
Adds Tenant.DefaultTenantId = "" and updates Tenant.Default.Id to use it, making the default-tenant ID convention explicit and dictionary-safe. |
src/modules/Elsa.Common/Multitenancy/Contexts/TenantResolverContext.cs |
Updates FindTenant to accept string? and normalize IDs before dictionary lookup, ensuring consistent behavior when callers pass null for the default tenant. |
src/apps/Elsa.Server.Web/Program.cs |
Enables multitenancy by default and installs the tenants feature via elsa.UseTenants() when useMultitenancy is true, wiring the new default-tenant behavior into the sample server (with a small style inconsistency on the new if statement). |
doc/adr/toc.md |
Extends the ADR table of contents with entries for explicit merge modes (ADR 6) and the empty-string default tenant ID (ADR 7), documenting the new architectural decision. |
doc/adr/graph.dot |
Updates the ADR dependency graph to include ADRs 6 and 7, but currently redefines node _4 with a different label/URL, making the graph inconsistent with the TOC and overriding the “Activity Execution Snapshots” label. |
doc/adr/0007-empty-string-as-default-tenant-id.md |
Adds a new ADR describing the move to "" as the default tenant ID, including context, decision, and consequences for the multitenancy subsystem. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…enancy condition formatting.
…modes, and default tenant ID - Introduced ADR 0005: Token-centric flowchart execution model for improved loop and join handling. - Added ADR 0006: Tenant Deleted event for distinct handling of tenant removal. - Documented ADR 0007: Explicit merge modes for flowchart joins, improving reliability and configurability. - Included ADR 0008: Standardization of empty string as the default tenant ID for consistency and clarity.
…invoker - Added comprehensive unit tests for tenant ID normalization to ensure consistent handling of null, empty, and valid IDs. - Introduced tests for the multitenancy pipeline invoker covering various tenant resolution scenarios. - Updated solution to include new unit testing projects for `Elsa.Tenants` and `Elsa.Common`.
- Refactor test parameterization to verify `HasExceptions` property more explicitly. - Simplify exception creation logic in helper methods. - Improve test assertions by combining act and assert phases where applicable.
- Introduced a configuration-based tenant provider to streamline tenant initialization and customization. - Added tenant ID handling filters to ensure tenant ID is applied and filtered automatically. - Deprecated the `CommonPersistenceFeature` in favor of modular persistence feature extension.
- Added `TenantId` to unique constraints on `Triggers` table across all EFCore providers. - Adjusted index names to reflect the updated constraints. - Updated trigger configuration to ensure uniqueness includes `TenantId`.
- Introduced `ITenantAccessor` to support tenant-specific filtering of workflow definitions. - Updated logic to skip workflows not matching the current tenant.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…kflowDefinitionStorePopulator`
… multitenancy - Added `TenantId` to unique constraints on the `Triggers` table and updated index names. - Included logic to drop outdated indexes without `TenantId` during migration. - Adjusted tests to account for `TenantId` in workflow identity and indexing scenarios.
…rigger indexing tests
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs: Language not supported
- Added `SelectiveMockLockProvider` to allow targeted lock mocking without affecting unrelated background operations. - Updated test services to use `SelectiveMockLockProvider` in place of `TestDistributedLockProvider`. - Refactored `DistributedLockResilienceTests` to support selective mocking for deterministic and reliable assertions.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs: Language not supported
src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs
Show resolved
Hide resolved
src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs
Outdated
Show resolved
Hide resolved
|
@sfmskywalker I've opened a new pull request, #7221, to work on those changes. Once the pull request is ready, I'll request review from you. |
src/modules/Elsa.Tenants/Services/DefaultTenantResolverPipelineInvoker.cs
Outdated
Show resolved
Hide resolved
…ed state handling - Updated `TenantResolverResult` to include an explicit `_isResolved` property. - Adjusted `ResolveTenantId()` and `IsResolved` logic for improved clarity and robustness. - Simplified tenant resolution invocation in `TenantResolverBase`. - Removed redundant normalization in `DefaultTenantResolverPipelineInvoker`.
…tor` and `ClrWorkflowsProvider`.
…bject initializations and add tenant-specific test coverage for `PopulateStoreAsync`.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 41 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Runtime/20251204150326_V3_6.Designer.cs: Language not supported
This change fixes an issue with workflow definitions and multitenancy support and standardizes tenant ID handling, updates database indexes, and modifies workflow definition loading to respect tenant boundaries.
An empty string (
"") is used as the default tenant ID. Backward compatibility is preserved through tenant ID normalization.Changes
ADR Documentation
Added ADRs for:
Tenant ID Normalization
NormalizeTenantId()extension method.nulltenant IDs to empty strings.Default Tenant ID
"") as the default tenant ID.Tenant.DefaultTenantId = "".Workflow Definition Loading
DefaultWorkflowDefinitionStorePopulatorto filter workflow definitions based on the current tenant.Database Index Updates
TenantId.Multitenancy Configuration
Program.cs.appsettings.json.Appsettings.json Update
appsettings.json.Unit Tests
Added unit tests for:
Impact
Behavioral Changes
Dependencies Affected
Potential Migration
nulltenant IDs should be migrated to empty strings.Breaking Changes
NormalizeTenantId()extension ensures backward compatibility for code that previously usednulltenant IDs.